Skip to content

Add new Tier-3 target: riscv64a23-unknown-linux-gnu #145076

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

ZhongyaoChen
Copy link

@ZhongyaoChen ZhongyaoChen commented Aug 8, 2025

MCP: Tier 3 target proposal: riscv64a23-unknown-linux-gnu

Changes:

  • add new target: riscv64a23-unknown-linux-gnu
  • add target page

@rustbot
Copy link
Collaborator

rustbot commented Aug 8, 2025

r? @jdno

rustbot has assigned @jdno.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-CI Area: Our Github Actions CI A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Aug 8, 2025
@rustbot
Copy link
Collaborator

rustbot commented Aug 8, 2025

These commits modify compiler targets.
(See the Target Tier Policy.)

Some changes occurred in src/doc/rustc/src/platform-support

cc @Noratrieb

@rustbot

This comment has been minimized.

@ZhongyaoChen ZhongyaoChen force-pushed the feature/add-tier3-riscv64a23-target branch from f77bc29 to 577b85a Compare August 8, 2025 03:41
@rust-log-analyzer

This comment has been minimized.

Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tier 3 targets do not get built nor tested in rust-lang/rust CI, and by implication, no prebuilt artifacts will be distributed for Tier 3 targets.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented Aug 8, 2025

☔ The latest upstream changes (presumably #145077) made this pull request unmergeable. Please resolve the merge conflicts.

@ZhongyaoChen ZhongyaoChen force-pushed the feature/add-tier3-riscv64a23-target branch from 64b98ad to 3ec3eea Compare August 11, 2025 08:21
@rustbot rustbot added the A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. label Aug 11, 2025
@rust-log-analyzer

This comment has been minimized.

@ZhongyaoChen ZhongyaoChen force-pushed the feature/add-tier3-riscv64a23-target branch from 3ec3eea to 1be21d3 Compare August 11, 2025 12:43
@rustbot
Copy link
Collaborator

rustbot commented Aug 11, 2025

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

This PR modifies tests/ui/issues/. If this PR is adding new tests to tests/ui/issues/,
please refrain from doing so, and instead add it to more descriptive subdirectories.

This PR changes how LLVM is built. Consider updating src/bootstrap/download-ci-llvm-stamp.

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

@rustbot rustbot added the T-clippy Relevant to the Clippy team. label Aug 11, 2025
@ZhongyaoChen ZhongyaoChen force-pushed the feature/add-tier3-riscv64a23-target branch from 1be21d3 to 87fd289 Compare August 11, 2025 13:10
@ZhongyaoChen ZhongyaoChen requested a review from jieyouxu August 11, 2025 14:59
@ZhongyaoChen
Copy link
Author

Ready for review. @zachs18 @Noratrieb @jieyouxu

@ZhongyaoChen
Copy link
Author

Pls review @jieyouxu @zachs18 @Noratrieb , i have make all changes requested.

@a4lg
Copy link
Contributor

a4lg commented Aug 18, 2025

I don't support adding support for the Supm extension because of its nature. At least, support in std_detect (backend of std::arch::is_riscv_feature_detected is once rejected by @Amanieu because of this (cf. rust-lang/stdarch#1770).

This extension itself does not provide any instructions, CSRs or machine behavior constraints but only denotes existence of a platform-dependent facility to control pointer masking behavior. For instance, the Supm extension on Linux denotes prctl-based control but we can actually call prctl to probe pointer masking.

EDIT: I don't support unless it's truly necessary (e.g. for interoperability with other object files compiled for RVA23U64). Even in this case, I would like to hear your thoughts about this extension.

@a4lg
Copy link
Contributor

a4lg commented Aug 18, 2025

And I have a concern regarding adding (so much) supervisor-mode extensions despite being a Linux target (which normally means a user mode target).

@a4lg
Copy link
Contributor

a4lg commented Aug 18, 2025

I personally recommend starting from RVA23U64 (excluding RVA23S64) and discuss possibilities extending it later.

@ZhongyaoChen
Copy link
Author

ZhongyaoChen commented Aug 18, 2025

I don't support adding support for the Supm extension because of its nature. At least, support in std_detect (backend of std::arch::is_riscv_feature_detected is once rejected by @Amanieu because of this (cf. rust-lang/stdarch#1770).

This extension itself does not provide any instructions, CSRs or machine behavior constraints but only denotes existence of a platform-dependent facility to control pointer masking behavior. For instance, the Supm extension on Linux denotes prctl-based control but we can actually call prctl to probe pointer masking.

EDIT: I don't support unless it's truly necessary (e.g. for interoperability with other object files compiled for RVA23U64). Even in this case, I would like to hear your thoughts about this extension.

Here are my thoughts:

  • Supm is mandatory in RVA23U64, so the rva23 target must support it.
  • HWASAN in LLVM may support RISCV in future, which need Supm. It may impact pointer address handling in LLVM backend code generation, so it must be explicitly passed to LLVM. Supm offers functionality nearly identical to aarch64 TBI, HWASAN in LLVM for aarch64 detailed on this page.
  • I think using a probe to detect this feature on OS is unrelated to whether this rva23 target should support it.

@jieyouxu jieyouxu removed A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. A-CI Area: Our Github Actions CI T-clippy Relevant to the Clippy team. labels Aug 18, 2025
@jieyouxu
Copy link
Member

r? compiler_leads

@rustbot rustbot assigned davidtwco and unassigned jdno Aug 18, 2025
Copy link
Member

@davidtwco davidtwco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Target spec LGTM. I don't have an opinion on the Supm feedback, @Amanieu do you?

r=me once @Amanieu has chimed in

@a4lg
Copy link
Contributor

a4lg commented Aug 18, 2025

@ZhongyaoChen Okay, for the "Supm" extension, that's convincing for me.

So, the remaining subjects I strongly oppose are RVA23S64 and its extensions (all new extensions starting with either Ss, Sh or Sv).

  1. This new target seems a user mode target with std (requiring RVA23U64, not RVA23S64) and implementing RVA23S64-specific supervisor mode extensions should not be a part of this proposal.
  2. Even if RVA23S64 extensions are necessary, we need:
    • Either new target or separate proposal (for existing bare-metal targets) and
    • Some additional supervisor extensions and implications (like "H", required by "Sha" but not implied from it). I'll review that in detail later.

@rustbot rustbot added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Aug 19, 2025
@ZhongyaoChen
Copy link
Author

@a4lg good point, changes applied.
RVA23S64 do need a separate kernel-level code target.

Copy link
Contributor

@a4lg a4lg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I request complete removal of RVA23S64 from this submission, not just from the target spec (including all RVA23S64-specific supervisor mode extensions).

Copy link
Contributor

@a4lg a4lg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. My minimal requirements are satisfied and seems almost ready for submission (we need to ask @Amanieu for the "Supm" extension though).

@ZhongyaoChen
Copy link
Author

Hi, This PR has been open for two weeks. All changes requested applied.
@davidtwco @Amanieu Pls help move this PR forward. Thanks.

@Amanieu
Copy link
Member

Amanieu commented Aug 22, 2025

The problem with Supm is that it doesn't actually enable any instructions or features on its own, so it is rather meaningless. I therefore don't think we should expose it as a user-visible target feature. Additionally, I checked and LLVM currently doesn't make use of it at all internally. We may revisit this once LLVM actually makes use of the Supm feature.

@0xllx0
Copy link

0xllx0 commented Aug 22, 2025

The problem with Supm is that it doesn't actually enable any instructions or features on its own, so it is rather meaningless. I therefore don't think we should expose it as a user-visible target feature. Additionally, I checked and LLVM currently doesn't make use of it at all internally. We may revisit this once LLVM actually makes use of the Supm feature.

From my understanding, it's meant to be a constraint on the execution environment, right? So, that would mean the user environment needs to provide the pointer masking.

Maybe conformance with Supm could be as simple as a check tied to the target triple? For example, something in the riscv64a23-unknown-linux-gnu target specification checks for some form of pointer masking syscall or similar? Alternatively, it could just be implicit that any target triple rustc provides with a riscv64a23- prefix must have some form of pointer masking.

I'm deferring to your and LLVM's opinion on this, though.

@Amanieu
Copy link
Member

Amanieu commented Aug 23, 2025

The problem is that "some form of pointer masking" isn't actually enough information to actually do anything. It doesn't tell you how many bits are masked, how to enable/disable masking, etc.

@a4lg
Copy link
Contributor

a4lg commented Aug 23, 2025

As a sidenote:

  1. Adding the support for the "Supm" extension changes object files (RISC-V attributes) to state that this extension is required on given object file.
  2. But is it worth it?
    • For tools that read RISC-V ELF attributes would see the difference.
    • However, the Linux kernel does not care about it when loading an executable.
    • It's still an RVA23-compliant program even if we don't require the "Supm" extension (because RVA23 profiles constrain execution environment).
    • std does not have any functions that changes behavior depending on the "Supm" extension, making this extension irrelevant inside the standard Rust (without external crates).

So my personal questions are:

  1. Does putting the "Supm" extension support in the RISC-V attributes help someone in the RISC-V ecosystem?
  2. Does supporting the "Supm" extension helpful for pointer masking users (like external crate developers to control pointer masking behavior if any)?

@ZhongyaoChen
Copy link
Author

The problem is that "some form of pointer masking" isn't actually enough information to actually do anything. It doesn't tell you how many bits are masked, how to enable/disable masking, etc.

@Amanieu

The pointer mask width is variable, depending on the virtual address width (Sv57, Sv48, Sv39), which represents the max, but a smaller width can be used in practice (e.g., Sv48 allows 16 bits, but we can only use 8 bits).

Additionally, this kernel patch specifies that userspace can enable/set/ask the number of mask/tag bits needed by the application using a syscall.

Additionally, I checked and LLVM currently doesn't make use of it at all internally. We may revisit this once LLVM actually makes use of the Supm feature.

Agreed.

But there's an extra issue: rva23u64 implies Supm support. In Rust, removing Supm but keeping rva23u64 is odd; passing rva23u64 to LLVM means Supm is needed, whether or not it's visible in Rust target features.

So my personal questions are:

Does putting the "Supm" extension support in the RISC-V attributes help someone in the RISC-V ecosystem?
Does supporting the "Supm" extension helpful for pointer masking users (like external crate developers to control pointer masking behavior if any)?

@a4lg Since LLVM doesn’t use Supm yet, sorry, these questions are unclear now.

@0xllx0
Copy link

0xllx0 commented Aug 23, 2025

The problem is that "some form of pointer masking" isn't actually enough information to actually do anything. It doesn't tell you how many bits are masked, how to enable/disable masking, etc.

Additionally, this kernel patch specifies that userspace can enable/set/ask the number of mask/tag bits needed by the application using a syscall.

This is what I meant about the Supm support being implicit inside Rust. The compiler itself doesn't do anything, like you said and the spec confirms, no machine instructions are output related to Supm. It's just a guarantee that whatever target environment is specified by the target triple supports pointer masking in some form.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.